-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Swap js integration #320
Conversation
sdk/js/src/svm.ts
Outdated
const expressRelayMetadata = getExpressRelayMetadataPda(chainId); | ||
const svmConstants = SVM_CONSTANTS[chainId]; | ||
|
||
const tokenProgramInput = TOKEN_PROGRAM_ID; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get from server
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the server parts. You may need to merge with main. There is a bunch of other changes merged recently.
e | ||
)) | ||
})?; | ||
let expected_router_fee_receiver_ta = Pubkey::find_program_address( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use this address for creating the permission key
// )) | ||
// }, | ||
// )?, | ||
router: self.config.chain_config.wallet_program_router_account, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we use the router_fee_receiver_ta
to create the permission key, we don't need to do these checks. actually the permission key will be invalid if the receiver_ta account does not match with what we expect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a good point, we should probably include the router in the permission_key
construction in get_quote_permission_key
. that's an easy fix, we can mark it as a TODO (or you can add it here after merging main)
e | ||
)) | ||
})?; | ||
let expected_router_fee_receiver_ta = Pubkey::find_program_address( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the whole point of making the router fee receiver ta generic as opposed to constraining it to be the ATA was so that routers could provide arbitrary token accounts
also the wallet_program_router_account is no longer in the config
// )) | ||
// }, | ||
// )?, | ||
router: self.config.chain_config.wallet_program_router_account, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a good point, we should probably include the router in the permission_key
construction in get_quote_permission_key
. that's an easy fix, we can mark it as a TODO (or you can add it here after merging main)
), | ||
tokenProgramOutput, | ||
mintOutput, | ||
routerFeeReceiverTa: getAssociatedTokenAddress( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not necessarily the ATA. the router account here will be the token account that the referrer wants the SPL fee token to be sent to, so you can just use that
@@ -0,0 +1,95 @@ | |||
import argparse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it would be easier (and better) to start writing the tilt scripts in JS? i think you could probably write this whole script more flexibly (using SDK methods rather than writing endpoints and payloads manually) in JS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, probably the simple ui we are going to make can be reused here too. But for the sake of wrapping this PR, I left it as it
expressRelayMetadata, | ||
searcher, | ||
trader: swapOpportunity.userWalletAddress, | ||
searcherInputTa: getAssociatedTokenAddress( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's better to make it optional variable?
If not exist set to getAssociatedTokenAddress
otherwise, use the input variable
mintInput, | ||
tokenProgramInput | ||
), | ||
searcherOutputTa: getAssociatedTokenAddress( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
), | ||
tokenProgramOutput, | ||
mintOutput, | ||
routerFeeReceiverTa: getAssociatedTokenAddress( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's better to make it Ata inside the program?
3589fc3
to
8a587d8
Compare
impl<T: ChainType> Service<T> { | ||
pub async fn get_opportunity_by_id( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better name here is get_live_opportunity_by_id
* The transaction in this bid transfers assets from the searcher's wallet to fulfill the limit order. | ||
* @param opportunity The SVM opportunity to bid on. | ||
* @returns The generated bid object. | ||
*/ | ||
async generateBid(opportunity: OpportunitySvm): Promise<BidSvm> { | ||
async generateBidLimo(opportunity: OpportunitySvmLimo): Promise<BidSvm> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, i still prefer limo and swap examples to live in two different files bc they are completely different functions
input_token: opp_sell_token.clone(), | ||
output_token: opp_buy_token.token, | ||
(_, 0) => QuoteTokens::OutputTokenSpecified { | ||
output_token: opp_sell_token.clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tiniest of nits, can you order input_token first for consistency? or output_token first is fine but let's just follow one order throughout
@@ -483,6 +494,188 @@ impl Service<Svm> { | |||
} | |||
} | |||
|
|||
async fn check_svm_swap_bid_fields( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i like packaging the validations all here, you could pass in SwapAccounts here to cut the redundancy, but i think it's fine this way too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's the responsibility of this function to extract such info.
.expect("Swap opportunity buy tokens must not be empty"); | ||
match (opp_sell_token.amount, opp_buy_token.amount) { | ||
(0, _) => QuoteTokens::InputTokenSpecified { | ||
output_token: opp_sell_token.token, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think output_token is the buy_token from the standpoint of the searcher
No description provided.